-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add coordtype
#167
base: main
Are you sure you want to change the base?
add coordtype
#167
Conversation
coordtype(::Point{<:Any,<:Any,<:AbstractArray{T}}) where T = T | ||
coordtype(::Point{<:Any,<:Any,<:NTuple{<:Any,T}}) where T = T | ||
coordtype(::Point{<:Any,<:Any,<:NamedTuple{<:Any,<:NTuple{<:Any,T}}}) where T = T |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove the Point
from here, so that the dispatch with parent(p)
for Point
feeds into these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that will hit the fallback method in fallbacks.jl. It just get the X coord type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also add some @test_inferred
? Or is it too early for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, good idea
GeoInterface implementations: Bridge packages
Readers
Other implementations
|
I'm fine with this, but might be good to give a concrete example of what this improves. Do we also want to give some hints using coordtype for let's say GI.coordinates? That's the most type unstable thing I can think of in here. |
Yeah, we could use it for (Answered the rest in the issue thread) |
Closes #128
Currently on a GI.MultiPolygon this takes 4ns. Its not a compile time operation. We could put this in the type parameters of the wrapper geometries at construction so its instant.
A lot of packages will be able to set this absolutely to a single type, e.g. Shapefile.jl, GeoJSON.jl and LibGEOS.jl can return
Float64
.